Repository navigation
Conversation
|
Status Reproduced on Bun 1.4.3 (Linux x64) and on a debug build of the base branch:
The new cases in This PR is stacked on #41617 and stays a draft until that PR merges and a maintainer answers the three decisions in the notes of the description. |
c01ebe1 to
9e9bdf3
Compare
|
Updated 3:08 PM PT - Oct 18th, 2026
✅ @robobun, your commit e55f201a988174d7bd2673b0ee6726ed61754906 passed in 🧪 To try this PR locally: bunx bun-pr 44612That installs a local version of the PR into your bun-44612 --bun |
|
cc @dylan-conway: this uses the path rule of #38365 for
An earlier version of this comment had a fourth point, a dangling symlink in the routes directory. #44968 settled it on main: the router and the resolver skip the link. The PR is stacked on #41617, because |
9e9bdf3 to
c0e964b
Compare
… not from Entry.abs_path Entry.abs_path is a slot of the shared directory-entry cache. The resolver fills it with the spelling of the first import that reaches a file. The router filled it with the kernel realpath and reused any value it found, so a route's filePath depended on whether the resolver saw the file first, and a router built first changed the path the resolver returned for later imports. Resolver::real_path_of is the rule that finalize_result and dir_info_uncached applied inline: the target of a symlinked entry, else the real path of its directory. The router builds each route's path from it. It no longer reads or writes Entry.abs_path, and it no longer opens each route file. The path and the route name share one interned string when the name is the tail of the path.
c0e964b to
e55f201
Compare
Problem
filePathdepends on earlier resolver calls. Through a symlinkeddir:<root>/real/pages/b.tsxalone,<root>/link/pages/b.tsxafterBun.buildof that file.reload()changes it again.Bun.buildread tsconfig.json from the real directory's parents.Entry.abs_path(src/resolver/fs.rs) is one slot of the shared directory cache. The resolver stores the first import's spelling.Route::parse(src/router/lib.rs) stored the kernel realpath and reused what it found.Fix
Route::parsetakes the path fromResolver::real_path_of, the rule thatfinalize_resultapplied inline (bake: serve routes behind symlinks, reload every route that shares a file #42813's hunk): a symlinked file's target, else the directory's real path plus the name. It no longer touchesEntry.abs_pathor opens route files.DirnameStoreslot per route per load, was 2.test/js/bun/util/filesystem_router.test.ts(15 new cases, 10 fail on the base), hot, watch and resolve suites. Self-reviewed: 12 concerns raised, 8 addressed, 4 sent to Decide a directory watch event from the watchlist, not from the resolver's cached listing #41617.Background
--hotfinds a file saved by rename through the slot that the router no longer fills. Decide a directory watch event from the watchlist, not from the resolver's cached listing #41617 reads the watchlist instead.get_fd_pathwithout the write: the kernel path ignores--preserve-symlinks, and Remove get_fd_path: derive paths from cwd and what was opened, not from fds #38365 removes that call.Downsides
ENOENT opening route.filePathkeeps the case ofdir, not of the disk.Notes
Decisions for a maintainer
Entry.abs_path.diron a case-insensitive file system:filePathfollowsdir, asBun.resolveSyncdoes. The old value had the spelling on disk. If the same file is also imported under the spelling on disk,import(filePath)loads a second copy of the module.reload()reads the disk.Settled on main: a dangling symlink
Main skips a dangling symlink in the router and in the resolver since #44968 (
EntryKind::Dangling, from #38756), and the base of this PR includes that commit. The router skips the entry beforeRoute::parseruns, so this PR does not change what happens to the link. One new test pins that the router lists the link only if the resolver resolves it. It passes on the base too.#34614 has a case that expects
ENOENT opening routefor a dangling symlink. Main no longer reports that error for a dangling link, and this PR removes it for every file.What still depends on order
filePathis now independent ofEntry.abs_path. It is the resolver's real path, so it follows the resolver in two cases:--preserve-symlinks, the resolver returns the link path before aBun.buildand the real path after one, because the bundler's resolver fillsDirInfo.abs_real_pathin the shared cache (Make --preserve-symlinks and --preserve-symlinks-main work in the runtime and bundler #36757). The router reports what the resolver returns in both states.A directory symlink that is created or retargeted after its parent was listed gives the old answer until the cache is busted. The router stays a reader of the resolver's cache here.
Reproductions on released 1.4.3 (Linux x64)
dirwithpages/c.tsxlinked to../shared/e.tsx: after a resolve the router reports<root>/pages/c.tsx, andreload()changes it to<root>/shared/e.tsx.Bun.resolveSync(pages + "/B", root)(wrong letter case) makes the router report<root>/pages/B.tsx, a file that does not exist on Linux.proj/tsconfig.jsonsetsjsxImportSource: "custom-src"andproj/linkpoints outsideproj.Bun.buildofproj/link/pages/b.tsximportscustom-src/jsx-dev-runtime. With a router overproj/link/pagesbuilt first it importsreact/jsx-dev-runtime.--preserve-symlinks: the router reports the real path withsrc../../real/pages/a.tsx, andBun.resolveSyncof the symlinked path returns the real path after the router ran.No user has reported these. The bug was found by a read of the code.
Measurements (debug builds of the base and of this PR, 100 and 1100 route files, per-route difference)
openat1,readlink1,close1reload()DirnameStoreappends per route, constructor (gdb breakpoint count)reload()The three syscalls are a few microseconds per route. They are not the reason for the design.
DirnameStoreis never freed and has a fixed number of slots, so one slot per route per load doubles the number of reloads before it is full. A route below a symlinked subdirectory, and a symlinked route file, still take a slot for the name.strace,perf,valgrindandbloatyare not installed here. The syscall count comes from a small ptrace counter and the append count from gdb breakpoints onBSSStringList<4096, 129>::do_append. Binary size is not measured: the change removes code and adds no host function.Other behavior changes
--preserve-symlinks,filePathis the path the resolver returns in that mode.srcis thena.tsxinstead of../../real/pages/a.tsx.ENOENT opening route: <root>/pages/a/b.tsx, because the old join treated the backslash as a separator. It is now listed as<root>/pages/a\b.tsx./procto load routes that are not symlinks.Related open PRs
--hotstops reloading a route file saved by rename afterrouter.reload(). The last new test covers that.Resolver::real_path_offor the bake route scan. Its resolver hunk is included here unchanged except for one doc line. Whichever PR lands second drops it.Entry::is_named()onEntry::abs_path(), which this PR deletes together withset_abs_path()because the router was their only caller. If resolver: a busted directory listing keeps its slot and is refilled in place #36675 lands second it adds the accessor again.Entry.abs_pathunder the per-entry mutex. With this PR the router no longer reads or writes that field.reload()over a symlinked directory, which mangles route names today. The new tests do not callreload()on a symlinked directory, except under--preserve-symlinks, where the directory keeps its link path.Not changed
bun test's scanner still usesEntry.abs_pathas its "already seen" mark, and the six resolver sites still fill it.Entry::symlink()still reads the cache without the mutex whilefinalize_resultstorescache.symlinkunder it. The router now reads through the same accessor as the resolver, so a fix there covers both.scopeguardstays insrc/router/Cargo.tomlalthough the crate no longer uses it, because dropping it also changesCargo.lock.Not run locally
Windows and macOS.
cargo checkpasses forx86_64-pc-windows-msvcandaarch64-apple-darwinon the touched crates. One new test needs a file system that ignores letter case and is skipped on Linux.no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/util/filesystem_router.test.ts